Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix use of timeout property for synchronous XMLHttpRequests #1390

Closed
wants to merge 1 commit into from
Closed

Fix use of timeout property for synchronous XMLHttpRequests #1390

wants to merge 1 commit into from

Conversation

bscuron
Copy link

@bscuron bscuron commented May 15, 2024

Convert XMLHttpRequests to be asynchronous to allow for the use of the timeout property. The timeout property can't be used for synchronous requests in a document environment (window/iframe) or an InvalidAccessError is thrown. (Please see the documentation)

This issue only appeared after updating my Salesforce org to the Summer '24 Release. I am deploying the Esri Leaflet map in Salesforce with LWC.

All npm test tests pass with the new changes.

Browser: Chrome
Browser version: 124.0.6367.207
Esri Leaflet version: 3.0.12
Leaflet version: 1.9.4
Bundling tool: Salesforce

@bscuron
Fix use of timeout property for synchronous XMLHttpRequests
054c031 
Convert XMLHttpRequests to be asynchronous to allow for the use of the
timeout property. The timeout property can't be used for synchronous
requests in a document environment (window/iframe) or an
`InvalidAccessError` is thrown.
@gavinr-maps
Copy link
Contributor

gavinr-maps commented May 15, 2024 •
edited
Loading

Hi @bscuron, thank you for the PR. I'm a bit confused why this is needed here - the MDN documentation for XMLHttpRequest.open says that the async property is true by default:
image
https://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequest/open

... so it seems like this PR is not changing anything. Could you maybe expand a bit more and provide a minimal demonstration case of the problem you're running into that this PR fixes? Thanks!

@bscuron
Copy link
Author

bscuron commented May 15, 2024

Hi @gavinr-maps, thank you for the response.

The Salesforce Summer '24 release most likely has a defect during LWC compilation where the third argument is not being treated as optional. By explicitly setting the third argument as true in open(), the library behaves as it did prior to the Salesforce Summer '24 release that occurred on May 11.

I plan to submit a support ticket to Salesforce to get the issue resolved. Would you consider merging this PR, since it does not affect users of the library in any negative way, but fixes an issue for users using the Salesforce Summer '24 release?

@jgravois
Copy link
Contributor

just my (unsolicited) two cents, but i'd consider a situation like this an ideal scenario to bundle a customized version of the library for yourself.

obviously there's overhead involved in tracking upstream changes any time you do this, but this library is pretty darn stable at this point.

@patrickarlt
Copy link
Contributor

@bscuron I think I agree with @jgravois here. Esri Leaflet is fairly stable at this point and it seems like this works in all of our target environments. If this is indeed a bug in Salesforce they should fix it and in the meantime you could use a custom build of this library until then.

@bscuron bscuron closed this by deleting the head repository Jun 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@bscuron @gavinr-maps @jgravois @patrickarlt